Skip to content

Conversation

@me-minus
Copy link

This commit changes the add puppet class view to be a bit more like a tree by changing the big + to be a right chevron or down chevron if the module has been opened.

When I have shown people how to create machines with the help of foreman, many have wondered what the difference between the '+' symbols when adding puppet classes are.

@nadjaheitmann
Copy link
Collaborator

Hi @me-minus ! Thanks for the contribution! Do you mind providing a screenshot that show cases your change?

@me-minus
Copy link
Author

foreman_with_chevrons

@nadjaheitmann
Copy link
Collaborator

Looks good to me and works fine. But I don't want to merge this without the consent of @adamruzicka .

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The erb is mixing tabs and spaces for indentation. Could you please only use spaces for that?

</ul>

<div class="panel-group paneless puppetclass_group" id="<%= list.first %>-avail-classes" style="margin-bottom: 1px;">
<div class="panel-heading" style="padding-top: 2px; padding-bottom: 2px;">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say I'm exactly happy about inline styling all over the place. Could that be extracted to classes?

.show();
.show()
.filter('div')
.find('a.collapsed')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the collapsed class being set on a <a>?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know.
I used the developers pane to find out how the attributes changed with some hints from the _classes.html.erb file.
The code was created by changing/playing with the left menu pane code in an older foreman version (probably before parternfly 5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants